Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vim: Expose the search-by-type feature #1846

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Julow
Copy link
Contributor

@Julow Julow commented Sep 27, 2024

This adds the :MerlinSearchType command to query for a value with a matching type. It works like the previous :MerlinSearch but uses the new search feature added in #1828, the search results are shown in the completion menu.

:MerlinSearch switches between polarity search and search by type depending on the first character of the query, like is done in the emacs plugin.

Documentation about the three search functions is added.

Copy link
Collaborator

@xvw xvw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

Copy link
Collaborator

@voodoos voodoos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Julow !

for prop in entries:
vim.command("let tmp = {'word':'%s','menu':'%s','info':'%s','kind':'%s'}" %
(prep(prop['name']),prep(prop['desc']),prep_nl(prop['info']),prep(prop['kind'][:1])))
vim.command("let tmp = " + vim_record({
Copy link
Collaborator

@voodoos voodoos Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unclear to me that vim_record behaves similarly as the previous use of prep and prep_nl. Does it "preserve newlines" ? And only for the "info" field ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, there was a problem here!
I've fixed vim_record to properly encode newlines and added back the prep and prep_nl functions. I'm not sure in which cases these are useful.
I've just learnt from this "info" feature and I find it awesome. I tested that completion works fine with these settings:

let g:merlin_completion_with_doc = 1
set completeopt+=popup

Comment on lines +304 to +312
" Do a polarity search or a search by type depending on the first character of
" the query.
function! merlin#Search(debug, query)
if a:query =~ "^[-+]"
call merlin#PolaritySearch(a:debug, a:query)
else
call merlin#SearchByType(a:debug, a:query)
endif
endfunction
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xvw this detection is also made on Merlin side isn't it ?
Maybe we could have a simpler interface in both emacs and vim:

  • Keep the existing PolaritySearch
  • Add a new SearchByType or Search, that does indeed also support the polarity search syntax.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in Emacs I've moved the original search behaviour into a search-by-polarity command and then the search command relies on the merlin search-by-type command (which classifies the query)

@voodoos
Copy link
Collaborator

voodoos commented Dec 9, 2024

Oh, I completely forgot about this one, sorry. @Julow is it ready ? 🙂

This adds the :MerlinSearchType command to query for a value with a
matching type. It works exactly like the previous :MerlinSearch, the
search results are shown in the completion menu.
':MerlinSearch' switches between polarity search and search by type
depending on the first character of the query, like is done in the emacs
plugin.

Documentation about the three search functions is added.
@Julow Julow force-pushed the vim-search-by-type branch from 321c7c9 to c742acc Compare December 10, 2024 10:05
@Julow
Copy link
Contributor Author

Julow commented Dec 10, 2024

I rebased and tested this in my own setup and it works perfectly.
Changelog updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants